[UI] Fix stale cache causing 'Table does not exist' error#4075
[UI] Fix stale cache causing 'Table does not exist' error#4075j1wonpark wants to merge 2 commits intoapache:masterfrom
Conversation
Signed-off-by: j1wonpark <jpark92@outlook.kr>
majin1102
left a comment
majin1102
left a comment
There was a problem hiding this comment.
Thanks for working on this. This is meaningful fix!
Left a comment(actually raised by AI), please take a look
| } | ||
| catch (error) { | ||
| const errorMessage = (error as Error)?.message || '' | ||
| const isNotFoundError = /not exist|not found/i.test(errorMessage) |
There was a problem hiding this comment.
Suggestions from AI:
There is a potential race condition in getTableDetails when the route changes while a previous request is still in flight. params is a computed value based on route.query, so during the await getTableDetail(...) call the user can switch from table A to table B, causing params.value to point to table B.
When the slow request for table A finally fails with "Table does not exist" / "table not found", the catch block reads catalogName, dbName, and tableName again from params.value. At that time params.value may already refer to table B, so we will:
- remove the cached table key from localStorage,
- emit tableNotFound for table B, and
- redirect the user back to /tables.
- This means a valid table B can be mistakenly treated as non-existent if a previous request for table A fails later, which is confusing for users.
Suggestion: take a snapshot of the parameters at the beginning of getTableDetails and use that snapshot consistently in both the request and the error handling, for example:
const getTableDetails = async () => {
isLoading.value = true;
const requestParams = { ...params.value };
const { catalogName, dbName, tableName } = requestParams;
try {
const res = await getTableDetail({
type: requestParams.tableType,
name: tableName,
database: dbName,
catalog: catalogName,
});
tableDetails.value = res?.data?.details;
getTableOptimizeInfo();
} catch (error: any) {
const e = error as AxiosError<any> & { data: { message?: string } };
const message = e?.data?.message;
if (message && (message.includes("Table does not exist") || message.includes("table not found"))) {
localStorage.removeItem(STORAGE_TABLE_KEY);
emit("tableNotFound", `${catalogName}/${dbName}/${tableName}`);
router.replace({ path: "/tables", query: {} });
return;
}
} finally {
isLoading.value = false;
}
};
There was a problem hiding this comment.
Thanks for the review! Fixed in f716e97 — params are now snapshot at the start of getTableDetails() so that both the API call and the catch block use the same values, preventing the race condition when the route changes during an in-flight request.
…request Signed-off-by: Jiwon Park <22048252+j1wonpark@users.noreply.github.com>
2 participants
Why are the changes needed?
When a table is deleted or no longer exists, the UI may still attempt to load it from stale cache (
localStorage), causing a "Table does not exist" error. This creates a poor user experience as the UI gets stuck trying to load a non-existent table.This fix ensures that when a table is not found:
localStorageBrief change log
amoro-web/src/views/tables/components/Details.vue:
tableNotFoundemit event to notify parent component when table doesn't existlocalStoragecache when API returns "not exist" or "not found" error/tablespage when table is not foundamoro-web/src/views/tables/index.vue:
handleTableNotFoundhandler to resetbaseInfostatetableNotFoundevent fromUDetailscomponentHow was this patch tested?
Run test locally before making a pull request
Add screenshots for manual tests if appropriate
Documentation